Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added simple Redis storage #38

Closed
wants to merge 35 commits into from
Closed

Added simple Redis storage #38

wants to merge 35 commits into from

Conversation

guillaumepotier
Copy link
Contributor

Hi there,

Following #37 here is a humble submission for a simple Redis storage.

I'm just starting with botkit (and thanks a lot) and I already know I cannot use file storage since my bots would live in various VMs.

I use node redis (npm install redis --save) and hashes sets (that have O(1) complexity like SET and GET and is a lot easier for all() method O(N) on the set vs. O(N) on the entire database for KEYS).

This is not tested in production (yet) and should not make any BC Break (just moved simple_storage.js into subdir if some other storages are going to be in the repo).

Tried to update a bit the README with my french background..

Please let me know if some things are wrong or missing, won't change my life if not merged, but could be nice for a broader adoption of the lib.

Best

@guillaumepotier guillaumepotier mentioned this pull request Dec 21, 2015
@guillaumepotier guillaumepotier changed the title Added simple Redis storage. Moved storage classes to storages/ subdir Added simple Redis storage Dec 21, 2015
@RafaelCosman
Copy link
Contributor

@guillaumepotier thanks very much for making this pull request! Can you make this storage system pass storage_test.js? I just wrote that so all of our storage systems will be tested and interchangeable. Thanks!

@guillaumepotier
Copy link
Contributor Author

Hi,

great to have some tests. I'm looking at them right now. I noticed thanks to the tests that all() methods returns an array. I find that not so handy since we deal with objects and users/teams/channels ids everywhere, my redis storage was returning an object like that :

{
  "id1": {"id": "id1", "foo": "bar"},
  "id2": {"id": "id2", "bar": "baz"}
}

I'll update my all() method than and maybe could I implement (for myself and the sake of others pushing the curiosity looking at the code) a allById().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants